Fix boolean options#500
Conversation
|
This seems quite complicated - would doing it like e.g. http://stackoverflow.com/a/15008806/2469283 be better? It seems to avoid the need for string mangling outside of argparser. |
|
I did it like that at first, but then I realized that the automatically generated help was hard to read. so I added groups for each pair of boolean options. I didn't go the t_or_f way because that's not the canonical way to express boolean options. Also I wanted to preserve existing options such as |
|
The muckery to allow both _ and - was probably the main thing that seemed messy to me. How would you feel about adding just the - versions and not supporting the underscores - I only did it that way in the first place because I wasn't familiar with this convention (I now understand that dashes do seem normal?). At this point, I think it's fine to do a small breaking change like this. Could you also pep8-style the code, there are just a few things like missing spaces after commas? |
|
if you are ok with the breaking change, then I am even happier. I'll update the PR accordingly. |
|
I made pep8 shut up, but there were lots of issues I wasn't responsible for :-) |
|
No problem, it was just pep8 of the new code I'm worried about, the rest is of course not your problem (actually I have to merge a tshirtman commit to fix them). |
|
they are all fixed in my PR (for |
|
Oh, in that case thanks even more! |
|
I had a query on #kivy-dev, but I guess I missed you. It was: The boolean option one seems fine, but how would you feel about stripping it to be simpler? There isn't actually really a need to support multiple args doing the same thing (presumably this stems originally from supporting the underscores?), and I also think that manually messing with the -- prefix is unnecessary. If that sounds reasonable to you, I can make those changes myself on merging, if you don't want to bother updating the PR. |
|
ah yes, I went to sleep. multiple names are still needed if you want to provide both long and short option names. I think tell me what you think. right now I have to go teach a class... I'm late :-/ |
|
I've merged this, thanks again for it. Regarding the stuff about argument formatting, I'll leave it for now but I think aim to consolidate everything at some point - right now there are also problems like ignored arguments and inconsistent argument names, so there should definitely be a break to fix it (and then after that keep it stable). |
type=boolis not the way to do boolean options withargparser. I addedadd_boolean_optionand used it to automatically define both--fooand--no-foooptions.